Conversation
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! Not a full review yet, but I left some comments inline which could change the approach in this PR.
|
|
||
| impl Blocklistable { | ||
| /// Component library path (merged account module name). | ||
| pub const NAME: &'static str = "miden::standards::components::utils::blocklistable"; |
There was a problem hiding this comment.
Similar to comments in other PRs, I don't think we should include components in the name here. But also, a couple of other comments:
- I think this should go somewhere under
faucetsnamespace (rather thanutilsnamespace) as this is applicable specifically to faucet accounts. - I'm not too sure about
blocklistableas the name because it has a slightly different meaning from other similarly-named components. For example,pausablemeans that the account itself can be paused, whileblocklistablemeans that other accounts can be blocked.
I don't have a great suggestion for the latter - maybe transfer_controls - but I don't love it either. Using it as a placeholder, the full name could be something like:
miden::standards::faucets::transfer_controls
|
|
||
| # The slot where the per-account blocklist map is stored. | ||
| # Map entries: [0, 0, account_id_suffix, account_id_prefix] => [is_blocklisted, 0, 0, 0] | ||
| const BLOCKLIST_SLOT = word("miden::standards::utils::blocklistable::blocklist") |
There was a problem hiding this comment.
For the slot name, I'd probably use something more specific - e..g, blocked_accounts or frozen_accounts etc.
| #! - the account is already blocklisted. | ||
| #! | ||
| #! Invocation: call | ||
| pub proc blocklist |
There was a problem hiding this comment.
nit: for the procedure names, we could probably use simple block, unblock, and is_blocked.
There was a problem hiding this comment.
I wonder if we should make this work more like the policy managers. In the current implementation, we just want to block/unblock accounts, but I can imagine more complicated transaction policies - e.g., limits on how much can be transferred in a given period of time, limits on transaction amounts, limits on who can recipients be etc.
So, maybe we need something like "Transfer Manager" and then one default transfer policy that only checks against the blocklist. And in the future, we could add more sophisticated policies.
If we do go with this approach, the component could be:
miden::standards::faucets::policies::transfer::policy_manager
And then this would go well with the naming for mint/burn:
miden::standards::faucets::policies::mint::policy_manager
miden::standards::faucets::policies::burn::policy_manager
2 participants
Closes the issue: